Conversation
Non-blocking follow-ups from the #654 re-review (no behavior change): - partnerPrincipal fields lowercased (partnerID/subject) — package-private type that never leaves the package. - Drop the no-op WithValidMethods on the ParseUnverified preview parse; add a comment that alg is enforced on the real jwt.Parse verify below. - Remove the now-unneeded `key := pk` loop copy (Go 1.22+ per-iteration binding). - Test: cover RawURLEncoding in the key-decode test (prod already accepts it).
dev-build/dev-agg/dev-op do not exist. The local gateway is run with make gateway (logs -> gateway.log) or make dev-gateway; operators via make dev-operator-sepolia; full local stack via make dev-stack.
The Run Unit Tests workflow's jobs (Check Changed Files, Lint, and the 13-package Unit Test matrix) mostly finish in under a minute, so Blacksmith's faster 4-vCPU runners have little wall-clock advantage there while still billing at the 4-vCPU rate on every push. Revert these jobs to ubuntu-latest, undoing #505 for this workflow only. The Docker image publish workflows (~4-5 min builds) stay on Blacksmith where the faster hardware genuinely pays off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Un7opV3NNydU4EEHD4pjAV
Analysis of the EigenLayer-AVS backend gaps for chat-driven, one-time single-node (contractWrite) execution — the preview→confirm→execute contract over nodes:run. Verified against the code with file:line refs: - G1: native ETH value dropped on the real path (PackExecute hardcodes 0) - G2: no typed tx identity / return value on real executes - G3: pending UserOp reported as failure; nodes:run is stateless - G4: approve+swap are non-atomic (2 UserOps), partial on-chain side effects - G5: execute is not idempotent (retried confirm re-broadcasts) - G6: runner salt not propagated on the nodes:run real path - G7: no execution fee on nodes:run (pricing decision) Scope: delegated execution (no per-tx user signature); server-side spend policy deferred; studio owns call construction + market-order helper + UX.
Co-authored-by: Chris Li <chris@avaprotocol.org>
…onse (#660) Co-authored-by: Chris Li <chris@avaprotocol.org>
Gateway-side response to studio's guardian-monitoring hand-off. Specifies the one missing capability as a generic primitive: durable cross-run per-WORKFLOW state (wfstate:<taskId>:...), scoped to taskId not to a user, so a user can run any number of monitoring workflows each with isolated, cascade-deleted state. Covers the SetIfAbsent/DeleteByPrefix storage additions (reusing IncCounter's serializable txn), the native guardian-scan node (holds GoPlus/Moralis creds, mints the GoPlus token, diffs via wfstate, claim-once alerts), credential config, and answers to the studio plan's five open questions. Additive storage (new namespace + model struct), no migration.
…ode) Replace the native guardian-scan node with two generic, reusable gateway extensions, verified feasible against the code: 1) REST node server-side auth providers (GoPlus signed-token minted from macros.secrets, injected into processedHeaders; secret never in client JSON) — precedents: options.summarize reader, balance runner reading macroSecrets, the /api/notify summarizer's own outbound call. 2) client-defined per-workflow state via a state.* goja binding over wfstate:<taskId> (client defines the JSON) — precedents: console.log Go-func binding, r.vm.db access, apContext AddVar injection. The guardian becomes a pure node composition (cron→REST→REST→customCode→ branch→notify) whose verdict/diff logic ports studio JS ~verbatim. Additive storage, no migration.
Position state as the client-defined read/write persistent namespace parallel
to {{settings.*}} (config) and {{context.*}} (read-only runtime): namespace
table, snake_case sub-key conventions, and the reservedSystemVarNames
registration requirement so it's re-injected fresh and collision-safe.
Add gateway-side commitments from the studio client review: (A) no in-place update confirmed → guardian_ruleset configVar for central rule updates + track an in-place UpdateWorkflow; (B) read-only create-auth relaxation via partner assertion as the Telegram-only enrollment fix, counterfactual runner accepted; (C) serialized + at-most-once + no-retry execution makes mark-after-send (at-least-once) the correct security bias, not a shortcut; (D) default cadence 6h, approvals-first v1.
Align gateway plan with the studio-side decisions (2026-07-15): A — accept the logic-freeze, no UpdateWorkflow ask; structural changes handled by re-create + silent-seed first run (no wfstate read API needed). B — web hand-off is the enrollment path (one EIP-191 sign ever, no create-auth change); partner- assertion relaxation parked as a future lever. Net gateway scope shrinks to the two extensions + guardian_ruleset configVar.
… hard-fails) Live run against v4.2.0 showed the verdict node fails on an unset guardian_ruleset configVar (could not resolve variable ... in source) before the JS runs — so it is required guardian setup, not optional-with-JS-fallback.
…662) Co-authored-by: Chris Li <chris@avaprotocol.org>
…uth extensions
ReviewReviewed the substantive Go changes (idempotent 1. GoPlus token mint holds a write-lock across a blocking network call (concurrency/perf)
Consider releasing the write lock before the network call and re-acquiring briefly to publish the result (accepting a handful of duplicate concurrent mints), or collapsing concurrent mints with 2. Idempotency cache entries are never actively evicted (unbounded storage growth)
Given BadgerDB doesn't appear to be wrapped with native per-key TTL support ( Everything else looked solid:
Nice test coverage overall — no blocking issues, just the two perf/resource-growth notes above. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the EigenLayer-AVS gateway/taskengine to support (1) durable per-workflow state accessible from CustomCode ({{state.*}}), (2) server-side REST auth providers (GoPlus) via restApi.config.options.auth, and (3) safer /nodes:run real contractWrite execution (native value passthrough, normalized receipt status, idempotency). It also includes supporting tests, documentation updates, and a CI runner change.
Changes:
- Add a durable per-workflow state store (
wfstate:<taskId>:...) with astate.get/set/listgoja binding and task-teardown cleanup. - Add REST
options.auth.providerparsing and GoPlus token mint/cache + Authorization header injection. - Make /nodes:run contractWrite execution safer/more usable (forward
value, normalize receiptexecutionStatus+userOpHash, idempotency viaIdempotency-Key, and surface per-method receipts in REST metadata).
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | Design record for composable workflow state + REST auth providers (guardian composition). |
| PLAN_AGENT_ONDEMAND_ACTIONS.md | Design/verification plan for on-demand single-node contractWrite execution gaps and fixes. |
| go.mod | Promotes golang.org/x/sync to a direct dependency (singleflight). |
| docs/changes/20260717-workflow-state-and-rest-auth.md | Shipped-change record for {{state.*}} and REST options.auth GoPlus provider. |
| core/taskengine/vm.go | Adds VM-scoped scratch backing for non-persistent state.* mode. |
| core/taskengine/vm_workflow_state_test.go | Tests for workflow state persistence/simulation-noop + REST auth provider parsing/injection. |
| core/taskengine/vm_runner_rest.go | Implements options.auth.provider + GoPlus token mint/cache and Authorization injection. |
| core/taskengine/vm_runner_customcode.go | Adds state.get/set/list binding for CustomCode (DB-backed or scratch). |
| core/taskengine/vm_runner_contract_write.go | Forwards native value on real path; stamps normalized executionStatus/userOpHash into receipts; adds sim executionStatus. |
| core/taskengine/vm_runner_contract_write_ondemand_test.go | Unit tests for native value forwarding and executionStatus/userOpHash behavior. |
| core/taskengine/schema.go | Adds WorkflowStateKey / WorkflowStatePrefix key builders. |
| core/taskengine/run_node_immediately.go | Adds Idempotency-Key singleflight + persistent TTL cache; propagates runner salt into aa_salt. |
| core/taskengine/run_node_immediately_idempotency_test.go | Tests idempotency replay/scoping/TTL behavior. |
| core/taskengine/macros/parseunit_test.go | Tests corrected ParseUnit scaling + fractional handling + rejection cases. |
| core/taskengine/macros/exp.go | Fixes ParseUnit to match ethers parseUnits semantics (scale by 10^decimals, fractional support). |
| core/taskengine/engine.go | Adds singleflight group to Engine; cascades delete of wfstate keys on workflow deletion. |
| config/test.example.yaml | Adds GoPlus secrets + guardian_ruleset examples for tests. |
| config/gateway.example.yaml | Adds GoPlus secrets + guardian_ruleset examples for dev gateway config. |
| CLAUDE.md | Updates stale make targets / dev commands documentation. |
| aggregator/rest/partner.go | Refactors partner principal fields to package-private; adjusts ParseUnverified usage; relies on Go 1.22 loop binding. |
| aggregator/rest/partner_test.go | Extends key-decoding test coverage (RawURLEncoding) and updates principal field access. |
| aggregator/rest/handlers_nodes.go | Wires Idempotency-Key into nodes:run; wraps array metadata under metadata.results. |
| aggregator/rest/handlers_nodes_test.go | Tests array metadata wrapping and map metadata passthrough. |
| .github/workflows/run-test-on-pr.yml | Moves unit-test workflow jobs back to ubuntu-latest runners. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3041
to
+3049
| // Idempotency needs a distinct authenticated subject to scope the cache key. | ||
| // A partner-assertion (simulate-only) caller can resolve to the zero address; | ||
| // deduping those would let one caller's key cross-replay another's result, so | ||
| // skip idempotency when there is no real subject. Safe by construction: real | ||
| // executes (fund-moving) always carry a non-zero authenticated owner. | ||
| if idempotencyKey == "" || user == nil || user.Address == (common.Address{}) { | ||
| return n.RunNodeImmediatelyRPCWithContext(ctx, user, req) | ||
| } | ||
| cacheKey := idempotencyCacheKey(user.Address.Hex(), idempotencyKey) |
Comment on lines
+57
to
+61
| var value interface{} | ||
| if err := json.Unmarshal(raw, &value); err != nil { | ||
| return goja.Undefined() | ||
| } | ||
| return value |
Comment on lines
+68
to
+71
| encoded, err := json.Marshal(value) | ||
| if err != nil { | ||
| return | ||
| } |
Comment on lines
+135
to
+144
| case []interface{}: | ||
| // Some node types (notably contractWrite) produce a per-method | ||
| // results array as metadata. RunNodeResponse.metadata is an object, | ||
| // so a bare array was previously dropped here — taking the per-method | ||
| // receipts (executionStatus, userOpHash, transactionHash) with it. | ||
| // Wrap it under "results" so those actually reach the client. | ||
| if len(v) > 0 { | ||
| wrapped := map[string]interface{}{"results": v} | ||
| out.Metadata = &wrapped | ||
| } |
- goplusAuthHeader: mint the token WITHOUT holding the cache write lock so an in-flight HTTP call (up to the client timeout) no longer serializes unrelated REST executions; publish under a brief lock afterward (last-writer-wins, duplicate concurrent mints are cheap/harmless). - state.get / state.set: log a Warn on JSON decode/encode failure instead of silently returning undefined / skipping the write, so corrupt or non-encodable state is diagnosable.
chrisli30
pushed a commit
that referenced
this pull request
Jul 17, 2026
…y-array metadata Addresses the three deferred review comments from #663: 1. Idempotency no longer dedupes SIMULATED contractWrite previews. A preview (is_simulated unset/true) skips the idempotency cache entirely, so a client reusing a key across a preview and a later real execute can't get the cached preview back in place of the broadcast. Only an explicit is_simulated=false contractWrite is deduped (contractWriteIsSimulated mirrors the runner default). 2. The idem:noderun: cache now uses a native Badger TTL (new Storage.SetWithTTL, backed by badger.Entry.WithTTL) so entries are reaped by the store even when a key is never re-read (the common fresh-key-per-action case) — bounding growth instead of relying on read-after-expiry deletion. The embedded timestamp + read-time check remain as a safety net. 3. runNodeRespToOpenAPI wraps an EMPTY results array as {results: []} instead of dropping it to metadata=null, so clients see a consistent shape for a legitimately empty result set. Also: gofmt trailing newline in version/version.go (left by the release bump). Tests: contractWriteIsSimulated resolution, empty-array wrapping. Build clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Non-blocking follow-ups from the #654 re-review (no behavior change):
partnerPrincipal fields lowercased (partnerID/subject) — package-private type
that never leaves the package.
Drop the no-op WithValidMethods on the ParseUnverified preview parse; add a
comment that alg is enforced on the real jwt.Parse verify below.
Remove the now-unneeded
key := pkloop copy (Go 1.22+ per-iteration binding).Test: cover RawURLEncoding in the key-decode test (prod already accepts it).
docs: fix stale make targets in CLAUDE.md
dev-build/dev-agg/dev-op do not exist. The local gateway is run with
make gateway (logs -> gateway.log) or make dev-gateway; operators via
make dev-operator-sepolia; full local stack via make dev-stack.
The Run Unit Tests workflow's jobs (Check Changed Files, Lint, and the
13-package Unit Test matrix) mostly finish in under a minute, so
Blacksmith's faster 4-vCPU runners have little wall-clock advantage
there while still billing at the 4-vCPU rate on every push. Revert
these jobs to ubuntu-latest, undoing #505 for this workflow only.
The Docker image publish workflows (~4-5 min builds) stay on Blacksmith
where the faster hardware genuinely pays off.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01Un7opV3NNydU4EEHD4pjAV
Analysis of the EigenLayer-AVS backend gaps for chat-driven, one-time
single-node (contractWrite) execution — the preview→confirm→execute
contract over nodes:run. Verified against the code with file:line refs:
Scope: delegated execution (no per-tx user signature); server-side spend
policy deferred; studio owns call construction + market-order helper + UX.
Co-authored-by: Chris Li chris@avaprotocol.org
Co-authored-by: Chris Li chris@avaprotocol.org
fix: parseUnit macro implements ethers parseUnits scaling with fractional support (fix: parseUnit macro implements ethers parseUnits scaling with fractional support #657)
docs: plan per-workflow state + native guardian-scan node
Gateway-side response to studio's guardian-monitoring hand-off. Specifies the
one missing capability as a generic primitive: durable cross-run per-WORKFLOW
state (wfstate::...), scoped to taskId not to a user, so a user can run
any number of monitoring workflows each with isolated, cascade-deleted state.
Covers the SetIfAbsent/DeleteByPrefix storage additions (reusing IncCounter's
serializable txn), the native guardian-scan node (holds GoPlus/Moralis creds,
mints the GoPlus token, diffs via wfstate, claim-once alerts), credential
config, and answers to the studio plan's five open questions. Additive storage
(new namespace + model struct), no migration.
Replace the native guardian-scan node with two generic, reusable gateway
extensions, verified feasible against the code:
macros.secrets, injected into processedHeaders; secret never in client
JSON) — precedents: options.summarize reader, balance runner reading
macroSecrets, the /api/notify summarizer's own outbound call.
wfstate: (client defines the JSON) — precedents: console.log
Go-func binding, r.vm.db access, apContext AddVar injection.
The guardian becomes a pure node composition (cron→REST→REST→customCode→
branch→notify) whose verdict/diff logic ports studio JS ~verbatim. Additive
storage, no migration.
Position state as the client-defined read/write persistent namespace parallel
to {{settings.}} (config) and {{context.}} (read-only runtime): namespace
table, snake_case sub-key conventions, and the reservedSystemVarNames
registration requirement so it's re-injected fresh and collision-safe.
Add gateway-side commitments from the studio client review: (A) no in-place
update confirmed → guardian_ruleset configVar for central rule updates + track
an in-place UpdateWorkflow; (B) read-only create-auth relaxation via partner
assertion as the Telegram-only enrollment fix, counterfactual runner accepted;
(C) serialized + at-most-once + no-retry execution makes mark-after-send
(at-least-once) the correct security bias, not a shortcut; (D) default cadence
6h, approvals-first v1.
Align gateway plan with the studio-side decisions (2026-07-15): A — accept the
logic-freeze, no UpdateWorkflow ask; structural changes handled by re-create +
silent-seed first run (no wfstate read API needed). B — web hand-off is the
enrollment path (one EIP-191 sign ever, no create-auth change); partner-
assertion relaxation parked as a future lever. Net gateway scope shrinks to the
two extensions + guardian_ruleset configVar.
Live run against v4.2.0 showed the verdict node fails on an unset
guardian_ruleset configVar (could not resolve variable ... in source) before
the JS runs — so it is required guardian setup, not optional-with-JS-fallback.
Co-authored-by: Chris Li chris@avaprotocol.org